[feature](routineload) Add routine load target-table alter support#64878
Open
0AyanamiRei wants to merge 23 commits into
Open
[feature](routineload) Add routine load target-table alter support#648780AyanamiRei wants to merge 23 commits into
0AyanamiRei wants to merge 23 commits into
Conversation
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: Allow paused single-table Routine Load jobs to switch their target table with ALTER ROUTINE LOAD FOR [db.]job ON <table>, while preserving existing progress and replaying the new table binding from edit log. ### Release note Support ALTER ROUTINE LOAD ... ON <table> to switch the target table for paused single-table routine load jobs. ### Check List (For Author) - Test: FE unit test - "/data/data3/huangruixin/include/src-master/apache-maven-3.9.9/bin/mvn -pl fe-core -am -DskipITs -Dcheckstyle.skip=true -DfailIfNoTests=false -Dtest=org.apache.doris.nereids.trees.plans.commands.AlterRoutineLoadCommandTest,org.apache.doris.persist.AlterRoutineLoadOperationLogTest,org.apache.doris.load.routineload.KafkaRoutineLoadJobTest test" - Kinesis unit tests skipped per user request - Behavior changed: Yes (new ALTER ROUTINE LOAD target-table switch behavior) - Does this need documentation: Yes (documented in /data/data3/huangruixin/docs/routine-load-alter-table-design.html)
0AyanamiRei
requested review from
924060929,
englefly,
gavinchou,
liaoxin01,
morrySnow,
mymeiyi and
starocean999
as code owners
June 26, 2026 04:59
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
0AyanamiRei
marked this pull request as draft
June 26, 2026 06:04
### What problem does this PR solve? Issue Number: None Related PR: apache#64878 Problem Summary: Follow-up review found two issues in the new routine load target-table alter support. First, `AlterRoutineLoadCommand` had an import order regression that could fail FE checkstyle. Second, alter validation only rechecked `PARTIAL_COLUMNS=true` from the current command, which left the effective partial-update state under-validated when the existing job or a `unique_key_update_mode` change required merge-on-write semantics. This change restores import ordering, validates the effective unique key update mode against the destination table, and adds focused FE unit coverage for those cases. ### Release note Routine Load alter now rejects target-table or unique-key-update changes that are incompatible with partial update requirements. ### Check List (For Author) - Test: Unit Test - Behavior changed: Yes - Does this need documentation: No
0AyanamiRei
marked this pull request as ready for review
June 29, 2026 01:11
Contributor
Author
|
run buildall |
### What problem does this PR solve? Issue Number: None Related PR: apache#64878 Problem Summary: The routine load target-table alter change hit FE checkstyle in CI because one validation error message exceeded the line-length limit and a unit-test import order did not match the FE custom import ordering rule. This commit makes the minimal formatting-only fixes so the branch aligns with FE style checks. ### Release note None ### Check List (For Author) - Test: No need to test (formatting-only fix requested by reviewer; no local build or test run) - Behavior changed: No - Does this need documentation: No
0AyanamiRei
changed the title
Contributor
|
/review |
### What problem does this PR solve? Issue Number: None Related PR: apache#64878 Problem Summary: The routine load target-table alter implementation had a single-use helper for constructing the validation load descriptor snapshot, and the parser branch for table-only alter did not mark the intended phase-one scope. This commit inlines the one-off snapshot construction at the validation call site and documents that the current parser branch only supports target table alteration before future support for combining target-table and property changes. ### Release note None ### Check List (For Author) - Test: No need to test (review follow-up only; no local build or test run) - Behavior changed: No - Does this need documentation: No
Contributor
Author
|
run buildall |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Review completed for PR 64878 at head 760890d.
I found one correctness issue: validation can accept an alter that later leaves the routine-load job in partial-update mode on a non-MoW table.
Critical checkpoints:
- Goal: the PR adds single-table routine-load target-table alteration while preserving stream progress; the implementation largely wires parser, command validation, job mutation, replay, display, and tests, but the partial-update validation path is inconsistent with mutation for one property combination.
- Scope/focus: the target-table change is small and localized to routine-load command/job/persistence paths.
- Concurrency/lifecycle: mutation is still gated by the existing paused-job write lock; no new background thread or lock order was introduced.
- Configuration/session propagation: no new config item; existing routine-load session/job properties are reused.
- Compatibility/persistence: new log field is backward-readable by current code; target-table id is replayed in Kafka and Kinesis paths.
- Parallel paths: Kafka and Kinesis both update/replay targetTableId; scheduling/task planning uses the current job tableId.
- Tests: parser/command/persistence/unit coverage and a Kafka regression were added. I verified
git diff --checkandmvn -pl fe-core -DskipTests checkstyle:check. I did not run FE compile/unit/regression tests because this checkout lacksthirdparty/installed/bin/protocandthirdparty/installed. - User focus: no additional user-provided focus was present.
Subagent conclusions: optimizer-rewrite proposed OPT-001, which became inline comment MAIN-001. tests-session-config reported no distinct candidate. Convergence round 1 ended with both live subagents replying NO_NEW_VALUABLE_FINDINGS for the same current ledger/comment set.
Contributor
|
Codex automated review failed and did not complete. Error: Codex completed, but no new pull request review was submitted for the current head SHA. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
TPC-H: Total hot run time: 29129 ms |
Contributor
TPC-DS: Total hot run time: 171607 ms |
Contributor
ClickBench: Total hot run time: 24.95 s |
### What problem does this PR solve? Issue Number: None Related PR: apache#64878 Problem Summary: ALTER ROUTINE LOAD validation handled unique_key_update_mode and legacy partial_columns with different precedence from the mutation path. An ALTER containing unique_key_update_mode=UPSERT and partial_columns=true could pass validation on a non-MoW table, then be applied as UPDATE_FIXED_COLUMNS. Flexible partial-update ALTERs on non-MoW tables were also rejected by the generic PARTIAL_COLUMNS validation before reaching the flexible partial-update validation path. This change makes generic partial-column validation apply only to fixed partial update mode and makes the mutation path ignore legacy partial_columns when an explicit unique_key_update_mode is present. The routine load alter regression test also declares its polling counters as local variables to satisfy regression framework script checks. ### Release note None ### Check List (For Author) - Test: Unit Test - ./run-fe-ut.sh --run org.apache.doris.nereids.trees.plans.commands.AlterRoutineLoadCommandTest,org.apache.doris.load.routineload.KafkaRoutineLoadJobTest,org.apache.doris.load.routineload.KinesisRoutineLoadJobTest - Behavior changed: No - Does this need documentation: No
Contributor
Author
|
run buildall |
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 29187 ms |
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 29668 ms |
Contributor
TPC-DS: Total hot run time: 177835 ms |
Contributor
ClickBench: Total hot run time: 25.06 s |
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
|
/review |
Contributor
There was a problem hiding this comment.
Requesting changes for four distinct correctness issues on the current head: the explicit FROM source type is discarded, target compatibility is checked against the old rather than effective job properties, and rejected combined ALTERs can leave unjournaled live mutations in both Kafka and Kinesis jobs.
Checkpoint conclusions:
- Goal and scope: the target-table switch is otherwise focused and the target-only Kafka/Kinesis plumbing, progress preservation, legacy-log zero default, and replay assignment are present. The four issues below prevent the advertised combined transition from being safe.
- Concurrency, lifecycle, and atomicity: paused-job locking and task clearing were traced through resume and transaction callbacks. Existing threads already cover mutable-target races; the two new source-specific findings below are distinct non-concurrent validation-to-commit failures.
- Validation and compatibility: TARGET remains non-reserved, ordinary string decoding is preserved, and target OLAP/non-temporary/single-table checks are present. However, the parsed source type is not preserved and same-statement effective update properties are omitted from target planning.
- Parallel paths and persistence: Kafka and Kinesis both assign and replay the new target ID, but both also mutate live source state before a throwable membership check and before the target/EditLog commit point, so a failed command diverges from durable state. Existing replay, image, Cloud, and journal-lock concerns were deduplicated against live threads.
- Tests and results: the changed tests cover matching parser forms, happy-path target switches, progress preservation, and legacy-log reading, but not these four negative cases. No local build or tests were run, as required by the review task. Live FE UT, compile, formatter, license, P0/nonconcurrent/external regression, performance, and vault P0 checks pass; cloud_p0 and check_coverage_fe are red, and their TeamCity details require authentication.
- Performance and observability: target validation adds planning only on explicit ALTER, not a scheduling hot path, and existing ALTER errors, job logs, SHOW state, and journal metrics remain available. No additional issue was found here.
- User focus: no extra focus points were supplied.
All three independent reviewers reread this exact four-comment set and returned NO_NEW_VALUABLE_FINDINGS on round 2. The live head and inline-thread set were refreshed immediately before submission, and no unresolved or duplicate candidate remains.
### What problem does this PR solve? Issue Number: N/A Related PR: apache#64878 Problem Summary: FE incremental coverage did not execute the real RoutineLoadJob target-table validation path and missed compatibility branches when ALTER contains no target table. The P0 scenario also combined target switching with unrelated property changes. Add focused unit tests for planner success and failure, lock release, the single-table invariant, legacy ALTER behavior, and Kafka/Kinesis no-target compatibility. Narrow the regression scenario to a target-only switch and an incompatible-target rejection. ### Release note None ### Check List (For Author) - Test: Unit Test and Regression test - ./run-fe-ut.sh --coverage --run relevant Routine Load test classes: 57 passed - ./build.sh --fe -j48: passed - Regression case added but not run locally because this worktree has no isolated BE/Kafka environment - Behavior changed: No - Does this need documentation: No
Contributor
Author
|
run buildall |
Contributor
|
/review |
Contributor
There was a problem hiding this comment.
Request changes: five distinct P1 correctness issues remain in the newly advertised combined target/source transition. Existing live threads already cover stale-target races, effective-definition validation, failed-ALTER atomicity, explicit source-type handling, and checkpoint/replay issues, so I did not duplicate them.
Critical checkpoint conclusions:
- Goal and scope: all 15 changed files were reviewed. The parser and target-only happy path work, but the combined transition is not yet safe across the Kafka/Kinesis parallel paths.
- Concurrency and lifecycle: paused-state and job-lock fencing were traced through scheduling and transaction callbacks. Existing target races are already covered; source identity/progress publication and replay still fail in the five cases below.
- Security and RBAC: current-job authorization precedes target privilege checking, and target LOAD authorization precedes metadata lookup. Existing discussions cover the remaining mutable-target authorization/state races.
- Persistence and compatibility: old operation logs safely default the new target ID to zero and both replay consumers are wired, but follower/image equivalence remains broken for Kinesis in the case below (in addition to already reported Kafka/checkpoint issues).
- Data correctness and parallel paths: Kafka and Kinesis can carry stale or synthetic offsets, shards, and caches across a newly committed target/source identity. The five inline comments identify distinct reachable cases.
- Tests and results: the added tests cover syntax, target-only success/failure, basic persistence, old-log defaulting, and ordered Kafka output, but not these source-identity, membership, default-preservation, or failover cases. This was a static review only; builds and tests were not run under the review contract.
- Configuration, protocol, performance, and observability: no new configuration, FE-BE protocol, hot-path, memory, or observability blocker was found. Any remote source validation should remain outside job locks and use a generation recheck before publication.
- User focus: none was supplied, so the complete PR was reviewed.
|
|
||
| modifyPropertiesInternal(jobProperties, dataSourceProperties); | ||
| if (command.hasTargetTable()) { | ||
| this.tableId = command.getTargetTableId(); |
Contributor
There was a problem hiding this comment.
[P1] Reset Kinesis state when the region changes. The source mutation below sets resetProgress only for a new stream name; an ALTER that keeps the same name but changes aws.region commits this new target and region while retaining progress, custom/open/closed shard IDs, and lag from the old physical stream. Pinned jobs then skip ListShards completely, and dynamic jobs can reuse an old sequence number when the new region exposes the same standard shard ID, producing an invalid starting position or repeated failures. Treat a region change as a source-identity change just like a stream change (or require validated explicit positions), and cover both pinned and dynamic jobs.
|
|
||
| AlterRoutineLoadJobOperationLog log = new AlterRoutineLoadJobOperationLog(this.id, | ||
| jobProperties, dataSourceProperties); | ||
| jobProperties, dataSourceProperties, command.getTargetTableId()); |
Contributor
There was a problem hiding this comment.
[P1] Validate explicit shards against the staged stream before journaling this transition. When kinesis_stream is supplied, modifyPropertiesInternal() clears progress, copies the requested IDs into customKinesisShards, and skips checkShards() because resetProgress is true. On resume that nonempty custom list also makes refreshKafkaPartitions() skip ListShards, so SET TARGET TABLE ... FROM KINESIS("kinesis_stream"="stream-b","kinesis_shards"="missing","kinesis_shards_pos"="LATEST") succeeds and persists a definition that will schedule a nonexistent shard. Resolve the effective endpoint/region/stream and fetch its actual shards outside the job lock, then recheck the job-definition generation and reject missing IDs before publishing the target/source delta.
|
|
||
| modifyPropertiesInternal(jobProperties, dataSourceProperties); | ||
| if (command.hasTargetTable()) { | ||
| tableId = command.getTargetTableId(); |
Contributor
There was a problem hiding this comment.
[P1] Re-seed progress when a broker-list change selects a different Kafka cluster. With the topic unchanged, modifyPropertiesInternal() only replaces brokerList; it retains progress, custom/current partitions, and the latest-offset cache. Pinned jobs then skip metadata discovery, and a dynamic job whose new cluster has the same numeric partition IDs appears unchanged, so tasks combine the new brokers and target here with offsets from the old cluster. Switching cluster A at offset 1000 to cluster B can therefore skip B's first 1000 records or pause with OFFSET_OUT_OF_RANGE. Resolve the cluster ID and any requested offsets against the effective new brokers outside the job lock; if the cluster changed, require those validated offsets or reset/reseed all progress and partition caches, persist that prepared decision for replay, and recheck the job generation before committing.
| Deencapsulation.setField(routineLoadJob, "progress", progress); | ||
|
|
||
| Map<String, String> sourceProperties = Maps.newHashMap(); | ||
| sourceProperties.put("property.client.id", "target-switch"); |
Contributor
There was a problem hiding this comment.
[P1] Preserve the existing Kafka default in this custom-only combined ALTER. Although this statement supplies only client.id, KafkaDataSourceProperties.analyze() falls through to analyzeKafkaDefaultOffsetProperty(), which synthesizes kafka_default_offsets=OFFSET_END; modifyPropertiesInternal() then merges that value into the live job before assigning and journaling the new target. A job created with OFFSET_BEGINNING can therefore execute the advertised SET TARGET TABLE ... FROM KAFKA("property.client.id"=...) form and silently make every later-discovered partition start at the end, skipping its backlog. This test masks the overwrite because the job starts with the implicit END default and never asserts it. Keep the default absent from an ALTER delta unless it was explicitly supplied (or needed for explicit partitions), and cover live/replay/image preservation from OFFSET_BEGINNING.
| modifyPropertiesInternal(log.getJobProperties(), | ||
| (KinesisDataSourceProperties) log.getDataSourceProperties()); | ||
| if (log.getTargetTableId() != 0) { | ||
| this.tableId = log.getTargetTableId(); |
Contributor
There was a problem hiding this comment.
[P1] Do not make Kinesis replay depend on follower-local shard discovery. Dynamic discovery adds shards to the leader's progress without journaling that discovery, so the leader can discover shard-0, pause before any task commit, and successfully journal an explicit shard position plus this target. A follower or older image with empty progress then replays the same record, checkShards() throws, this catch consumes the journal entry, and the tableId assignment here is skipped. The new replay test cannot exercise this because its captured record has no explicit shard positions (and it also pre-seeds shard-0). Replay a fully prepared leader-validated transition without consulting local progress (or persist the required canonical state in the same record), and add the empty-progress failover/image case.
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
TPC-H: Total hot run time: 30292 ms |
Contributor
TPC-DS: Total hot run time: 177484 ms |
Contributor
ClickBench: Total hot run time: 25.04 s |
### What problem does this PR solve? Issue Number: N/A Related PR: apache#64878 Problem Summary: Merge the latest master into the routine load target-table branch. Resolve the parser conflict by reusing master SqlLiteralUtils for TARGET TABLE string decoding while preserving the target-table ALTER behavior. ### Release note None ### Check List (For Author) - Test: Unit Test - ./run-fe-ut.sh --run org.apache.doris.nereids.trees.plans.commands.AlterRoutineLoadCommandTest - Behavior changed: No - Does this need documentation: No
Contributor
|
/review |
Contributor
There was a problem hiding this comment.
Request changes: two additional P1 correctness issues remain at head 04a5e1085b5b7f03d54a3abdafa9fd9a72ccf182. The new composed target-plus-partial_columns form looks up the database name as the job label, and a successful Cloud target-plus-custom-Kafka ALTER deletes the authoritative durable progress. Existing inline concerns were deduplicated and are not repeated here.
Critical checkpoint conclusions:
- Goal and proof: The patch adds
SET TARGET TABLE, target compatibility planning, Kafka/Kinesis live assignment, EditLog replay, legacy-log decoding, unit tests, and a Kafka target-only P0 case. The happy path is present, but the two inline issues show that advertised job/source composition and progress preservation are not yet correct. - Scope and clarity: The 15 changed paths are focused on parser, command validation, Routine Load state, persistence, and tests. Reusing the existing source-property mutators nevertheless crosses planner state, source identity/progress, failover, and image/replay boundaries.
- Concurrency: The per-job write lock serializes final in-memory apply, but command validation is outside that snapshot and Cloud progress is not atomically coupled to the target/EditLog transition. Existing target/job/descriptor races are already covered by current threads; the new Cloud comment covers the remaining successful-transition failover window.
- Lifecycle: PAUSE clears active task objects before ALTER, so no distinct simultaneous old/new-target task bug was found. Resume, discovery, default seeding, task creation, transaction progress, failover, replay, and checkpoint reconstruction were traced; existing threads cover the other lifecycle defects.
- Configuration: No configuration item or dynamic-config behavior is added.
- Compatibility and persistence: A legacy operation log without
targetTableIddecodes to the zero sentinel, and current Kafka/Kinesis replay assigns nonzero target IDs. Existing threads already cover state-dependent replay and stale-CREATE checkpoint reconstruction. The second inline comment identifies a separate successful-path deletion of Cloud durable progress. - Parallel paths and special conditions: Kafka/Kinesis, target-only/source-only/composed forms, pinned/dynamic partitions or shards, explicit/default/empty progress, standard/Cloud Kafka, and live/replay/image paths were compared. Cross-type
FROM, source-identity resets, default synthesis, failure atomicity, and replay membership checks are already covered by existing threads. - Test coverage and results: The changed tests cover parser constraints, mocked target validation, live target assignment, target-only replay, old-log decoding, and one ordered Kafka target-only regression. They do not cover target plus legacy
partial_columnsor Cloud MetaService progress plus failover, directly masking the two comments. The changed.outmatches the ordered regression queries. No build or test was run in this review because the governing review prompt forbids builds/tests. - Observability and error handling: Existing Routine Load and replay logs are sufficient for this administrative path; the unchecked null/wrong-job path and the missing durable progress are correctness defects rather than missing diagnostics.
- Data correctness and writes: No BE row-write implementation changes, but Routine Load offsets decide which records reach the new target. The Cloud deletion can skip or reconsume data after failover.
- FE/BE propagation: No new FE-to-BE protocol variable is introduced; task planning already resolves the job's current table ID and existing source fields.
- Performance: Target planning is ALTER-time control-plane work, not a hot loop. No distinct CPU, memory, or hot-path regression was found; existing metadata-lock I/O/journal concerns were deduplicated.
- Other issues and user focus: No additional user-provided focus was present. Every suspicious point has a main-agent status, and the final 15-file/unresolved-risk sweep found no third distinct issue.
Review convergence: Round 1 found these two candidates. After main verification and live duplicate refresh, both normal full-review lanes and the separate risk lane repeated a complete Round 2 review and each returned NO_NEW_VALUABLE_FINDINGS for this exact head, 33-comment context, bodies, severities, and anchors. Validation was static only as required.
### What problem does this PR solve? Issue Number: N/A Related PR: apache#64878 Problem Summary: The target-table ALTER path still accepted Kinesis jobs and persisted target-table IDs through Kinesis mutation and replay, which expanded the feature beyond its intended Kafka-only scope and exposed unrelated Kinesis source-transition issues to this PR. Reject SET TARGET TABLE for non-Kafka jobs, remove the Kinesis target-table mutation and replay changes, and replace the Kinesis positive test with a Kafka-only rejection test. Existing Kinesis property-only ALTER behavior remains unchanged. ### Release note ALTER ROUTINE LOAD SET TARGET TABLE supports Kafka Routine Load jobs only. ### Check List (For Author) - Test: Unit Test - ./run-fe-ut.sh --run org.apache.doris.nereids.trees.plans.commands.AlterRoutineLoadCommandTest,org.apache.doris.load.routineload.KafkaRoutineLoadJobTest,org.apache.doris.load.routineload.KinesisRoutineLoadJobTest - Behavior changed: Yes. Kinesis jobs now reject SET TARGET TABLE while existing property-only ALTER remains supported. - Does this need documentation: Yes. apache/doris-website#3988
Contributor
Author
|
run buildall |
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
TPC-H: Total hot run time: 29643 ms |
Contributor
TPC-DS: Total hot run time: 180734 ms |
Contributor
ClickBench: Total hot run time: 25.12 s |
Contributor
|
/review |
Contributor
There was a problem hiding this comment.
Request changes.
I found one new P1 mixed-version persistence issue (inline): an older electable FE can successfully replay the known ALTER ROUTINE LOAD opcode while dropping the new target-table field, then resume ingestion into the old table after failover. I did not duplicate the existing threads covering effective-property planning, validation/commit races, Kafka/Cloud progress atomicity, same-version replay/image drift, explicit source-type loss, or mutable-table authorization/existence races.
Critical checkpoint conclusions:
- Goal and scope: the focused 13-file patch implements the Kafka target-switch syntax, live happy path, and same-version target-only replay, but the feature is not end-to-end safe until the mixed-version journal gap and the already-raised correctness blockers are resolved.
- Concurrency: the live assignment is serialized by the job write lock and the PAUSED task/commit lifecycle did not expose an additional leak. Existing threads already cover the validation-to-commit generation gaps and unsynchronized mutable-table consumers.
- Lifecycle, persistence, transactions, and data writes: old-log -> new-reader compatibility and same-version target-only replay are sound. New-log -> old-reader replay is not: failover can route future Kafka transactions and plans to the old target. Existing threads separately cover follower-local progress, checkpoint reconstruction, and pre-journal Cloud side effects.
- Compatibility and parallel paths: Kafka-only target switching is enforced and Kinesis property-only behavior remains parallel by design. No configuration or FE-BE protocol field is added; the reused FE journal record is the blocking compatibility boundary.
- Conditions, errors, observability, and performance: target validation performs one planner construction per ALTER and adds no recurring hot-path work. I found no separate new observability or error-propagation issue beyond the existing review threads.
- Tests and results: the changed unit/regression tests cover parsing, target constraints, live target-only/combined mutation, planner unlock, same-version replay, old-log deserialization, and deterministic future-record routing. They do not cover mixed-version replay/failover. Live CI shows compile, FE UT, P0, non-concurrent, external, Cloud, vault, and performance checks passing;
check_coverage_feis failing. No local build or test was run because this review workflow explicitly forbids builds. - Documentation and focus: the linked documentation PR exists and describes the Kafka-only scope. No additional user-provided review focus was supplied.
Overall, the PR is not ready to merge while these data-routing and persistence issues remain.
| @SerializedName(value = "dataSourceProperties") | ||
| private AbstractDataSourceProperties dataSourceProperties; | ||
| @SerializedName(value = "targetTableId") | ||
| private long targetTableId; |
Contributor
There was a problem hiding this comment.
[P1] Prevent old FE followers from silently dropping the target switch. This field is added to the existing OP_ALTER_ROUTINE_LOAD_JOB JSON record, so an older electable FE recognizes the opcode and replays the known empty job/source delta, while Gson ignores the unknown targetTableId and the old Kafka replay path never changes tableId. During a rolling upgrade, if the old master fails after one upgraded follower rejoins, that new binary can become master while another follower still runs the old binary; after it switches a paused job from A to B, failover to the old follower makes beginTxn and task planning use A's ID and sends the continued stream back to A. The missing-field test covers only old-log -> new-reader compatibility, not this new-writer -> old-reader direction. Gate this ALTER until every electable FE advertises support, or use a versioned record/protocol that an old FE cannot silently accept, and add a mixed-version replay/failover test.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: N/A
Related PR: N/A
Problem Summary:
Routine Load target-table alteration was initially limited to the target-only syntax
ALTER ROUTINE LOAD FOR [db.]job ON table. It could not alter supported job or Kafka data-source properties in the same statement.This PR replaces that syntax with the explicit, composable form:
The implementation has the following behavior and constraints:
SET TARGET TABLEis supported only for Kafka Routine Load jobs. Kinesis jobs reject target-table changes, while their existing property-only ALTER behavior remains unchanged.COLUMNS,WHERE, andPARTITION.LOADprivilege checks, and be compatible with the existing load definition and distribution mode.Release note
ALTER ROUTINE LOADnow usesSET TARGET TABLE = "table"to switch the target table for Kafka Routine Load jobs and supports combining the switch with supported job and Kafka data-source properties.Check List (For Author)
SET TARGET TABLE = "table"for target-table switching. Kinesis target-table switching is not supported.Check List (For Reviewer who merge this PR)